-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Range verification should not fail media responses #7782
Conversation
Content-Range
values in range responses and allow 200
Source for WebKit not opposed? Last response from Jer Noble (developer of the multimedia stack on WebKit's Apple ports) in the bug you linked (https://bugs.webkit.org/show_bug.cgi?id=211323) clearly says:
Alicia here, maintainer of the GStreamer ports (WebKitGTK and WPE): I still sustain servers without range request support are ill-suited for media serving. The reasons for that are:
For all the previous reasons, media playback without Range-Request support in the server -- even with the most fully featured workarounds -- is not possible without significant performance degradation for the user, wasted data usage for the user, the server and the network. It adds significant complexity to the user agents, while encourages developers and server maintainers to forget best practices. Last thing I want is to encourage CDNs and server maintainers to pay even less attention to Range-Request support. I oppose. |
Apologies, I totally missed that comment. Not sure whether this is meant as an opposition or a limitation but I will let Apple speak for that.
Thanks for this view, Alicia. All of the comments above are about servers with no range support at all, which is part of the PR. The other part is about not requiring that the range I. the request and response match, which is not an http requirement and I am not sure if the Apple comment is relevant to that. Could you share your view on that? I am not sure if the Apple or GStreamer ports currently perform this verification. |
As a way to reflect current state of implementations, I suggest to put the 200/non-matching-range as a MAY and make the WPTs tentative. I wish though that this can work towards a consensus, the media part of HTML is quite tentative as it is. |
Content-Range
values in range responses and allow 200
We don't like optional features in our specifications. Let's align with 2/3 browsers. |
Done. |
I am opposed. Requesting a specific Range and receiving an entirely different Content-Range in response should be treated as an error. UAs can decide to be lenient in the face of a buggy or misconfigured server, but we should not enshrine that behavior as supported in a specification. |
Understood. We can have a tracking issue open for seeing if other browsers are willing to align with Safari's behavior. For now the most important priority is to get the spec to reflect reality. |
In this case, we can't have the spec both avoid optional behavior and reflect reality. My original question around range requests came from a view that, while consistent with historic WebKit behavior, was apparently not what was intended. Although "one or more parts" might imply that the server MAY partially respond to the range request, this other sentence might imply that providing only part of what is requested should be limited to situations where the other parts are not available due to the "current length of the representation": Questions around the server providing only part of the requested range were raised on ietf-http-wg and the responses indicate that this was not intended, not "designed for", and perhaps would have been considered poor design. So the remaining part of my range request question then is whether there is need for the inconsistency of the case where Plausibly the complete-length may not be known at the time of generating the header field if the Range request has a last-byte-pos less than the "current length of the representation". However the Content-Range header in the response to a "Range: bytes=0-" request will include the last-byte-pos corresponding to the last byte in the representation, and so complete-length will be known. @jakearchibald wrote the "Validate a partial response" algorithm for background-fetch and so may be interested in this discussion. |
A 200 (OK) response is clearly a valid response, and reasonable at least for a smaller media resource. Is the acceptance of that less contentious? |
From the POV of HTTP, it's valid. For all the reasons @ntrrgc mentions above, a server responding to a Range request with a 200 response breaks a number of important media use cases. I would like to avoid the specification implying that a UA which requires a 206 response to a Range request for media is somehow "out of compliance".
This is verging on "how many hairs make a beard" territory. How small a resource is small enough to be reasonable? |
That was the basis for the current algorithm, though it's a bit nuanced because of the multiple-origin verification. |
I believe WebKit does a sensible thing by sending a Regarding |
Despite the questionable validity of incomplete Partial Content responses, Gecko implemented support for requesting the missing portion after a few compatibility reports. If incomplete responses to I can imagine that having the trailing portion of the range missing might be easier to support than having the leading portion of the range missing, because the leading portion is usually needed first and the trailing portion may also be unavailable for other reasons such as network timeouts. FWIW Gecko seems to re-request if either portion is missing. |
So to summarize:
I believe that the right way forward is to put wording that discourages 200 responses and range mismatch, and that though they are permitted, some UAs can fail them for performance reasons (e.g. if downloading the whole response and buffering it on the client side is too costly network-wise) |
I don't think we need to add anything to the spec to allow WebKit's behavior. The spec does not mandate that all network requests succeed---it cannot, because sometimes networks go down or have errors. If 1% of requests happen to fail in WebKit and not in other browsers, that seems very reasonable. As for whether the spec should discourage 200s, I'm not sure. That doesn't feel like something for the HTML spec to adjudicate; it feels like it belongs in the HTTP specs for Range requests instead? |
I think it should discourage those responses, not because of webkit behavior, but because it's a rare anti-pattern that webkit chose to not support. Discouraging range mismatches belongs in the HTML spec because the problematic implications are on how user-agents handle media resources, which is an application-level concern rather than a protocol problem. In other implementations/uses of range headers this might be totally OK, e.g. a media application like VLC can choose to download the whole resource and perform buffering on the client side if the server doesn't support range requests. |
@domenic: the current revision of this PR allows range mismatches, while discouraging them for the aforementioned reasons. How do we proceed? |
I still don't feel comfortable discouraging 200 responses to range requests in HTML, when it isn't discouraged for HTTP in general. Maybe that part of the PR could be separated out and subject to a larger discussion with the HTTP + browser media community. |
Verifying media responses performs 3 verifications: * The response should also be a range response * The response's range should match the request's range * There should not be mixed origins with opaque response While the last verification is mandatory for same-origin policy, the first two are optional, and may be skipped if the user-agent decides to be lenient towards servers who don't handle range requests properly. See whatwg#7655 (comment)
OK, I removed that line for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to opening a dedicated issue about whether to add guidance (or update browsers) for the 200 responses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM assuming the tests verify that syntactically invalid range headers are ignored.
Added requisite tests: web-platform-tests/wpt#33990 |
Looks like Firefox fails those; can you file a bug and update the OP? |
Done |
@domenic, anything mssing? |
I don't understand something. You remove the dependency on parsing the However, https://bugzilla.mozilla.org/show_bug.cgi?id=1768448 suggests that Firefox not validating the range response according to |
Still parsing the header, but allowing 200 responses and responses that don't match the requested range. Syntactically invalid responses are still rejected. |
How do they end up rejected if we don't look at the |
I see what you mean, in one of the iterations we removed an important check. We have to check that the range is valid (even if not matching). |
Revised, now extracting content-ranges is expected to at least succeed, even if the range itself is not validated. This aligns with the WPTs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good after adding a note explaining the (somewhat strange) situation.
Some User-agents don't actually perform these verifications.
The only mandatory verification is origin of responses.
See #7655 (comment)
At least two implementers are interested (and none opposed):
Tests are written and can be reviewed and commented upon at:
Implementation bugs are filed:
(See WHATWG Working Mode: Changes for more details.)
/infrastructure.html ( diff )
/media.html ( diff )